From b6d1119f912059b4851e1c37941eb176f8b5b192 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 27 Jan 2022 16:37:30 -0500 Subject: [PATCH] tests: Stop using inventory crate I was reading this thread https://internals.rust-lang.org/t/from-life-before-main-to-common-life-in-main/16006/30 and that reminded me about this code, which it turns out actually doesn't compile with my default local cargo config: ``` $ cat ~/.cargo/config [target.x86_64-unknown-linux-gnu] rustflags = ["-Ctarget-cpu=native", "-C", "link-arg=-fuse-ld=lld"] [profile.release] incremental = true $ cargo b ... error: linking with `cc` failed: exit status: 1 | = note: "cc" "-m64" "/var/srv/walters/src/github/ostreedev/ostree/target/debug/deps/ostree_test-4ca8e730f9dc6ffc.10325uqlhkyr5uol.rcgu.o" "/var/srv/walte" = note: ld.lld: error: undefined symbol: __start_linkme_NONDESTRUCTIVE_TESTS >>> referenced by 22nn09lfsklfqvyy >>> /var/srv/walters/src/github/ostreedev/ostree/target/debug/deps/ostree_test-4ca8e730f9dc6ffc.22nn09lfsklfqvyy.rcgu.o:(ostree_tes) ``` For now let's just go back to having a static list of functions. We don't have *too* many of those. --- tests/inst/Cargo.toml | 4 -- tests/inst/itest-macro/Cargo.toml | 14 ----- tests/inst/itest-macro/src/itest-macro.rs | 71 ----------------------- tests/inst/src/destructive.rs | 3 +- tests/inst/src/insttestmain.rs | 44 +++++++++----- tests/inst/src/repobin.rs | 15 ++--- tests/inst/src/sysroot.rs | 9 +-- tests/inst/src/test.rs | 16 +---- 8 files changed, 40 insertions(+), 136 deletions(-) delete mode 100644 tests/inst/itest-macro/Cargo.toml delete mode 100644 tests/inst/itest-macro/src/itest-macro.rs diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index 146744b8..31303f72 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -29,7 +29,6 @@ futures-util = "0.3.1" base64 = "0.12.0" procspawn = "0.8" rand = "0.7.3" -linkme = "0.2" strum = "0.18.0" strum_macros = "0.18.0" openat = "0.1.19" @@ -40,6 +39,3 @@ rpmostree-client = { git = "https://github.com/coreos/rpm-ostree", tag = "v2021. # This one I might publish to crates.io, not sure yet with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-tempdir" } - -# Internal crate for the test macro -itest-macro = { path = "itest-macro" } diff --git a/tests/inst/itest-macro/Cargo.toml b/tests/inst/itest-macro/Cargo.toml deleted file mode 100644 index 54494d29..00000000 --- a/tests/inst/itest-macro/Cargo.toml +++ /dev/null @@ -1,14 +0,0 @@ -[package] -name = "itest-macro" -version = "0.1.0" -edition = "2018" - -[lib] -proc-macro = true -path = "src/itest-macro.rs" - -[dependencies] -quote = "1.0.3" -proc-macro2 = "1.0.10" -syn = { version = "1.0.3", features = ["full"] } -anyhow = "1.0" diff --git a/tests/inst/itest-macro/src/itest-macro.rs b/tests/inst/itest-macro/src/itest-macro.rs deleted file mode 100644 index 34d35a1a..00000000 --- a/tests/inst/itest-macro/src/itest-macro.rs +++ /dev/null @@ -1,71 +0,0 @@ -extern crate proc_macro; - -use proc_macro::TokenStream; -use proc_macro2::Span; -use quote::quote; - -/// Wraps function using `procspawn` to allocate a new temporary directory, -/// make it the process' working directory, and run the function. -#[proc_macro_attribute] -pub fn itest(attrs: TokenStream, input: TokenStream) -> TokenStream { - let attrs = syn::parse_macro_input!(attrs as syn::AttributeArgs); - if attrs.len() > 1 { - return syn::Error::new_spanned(&attrs[1], "itest takes 0 or 1 attributes") - .to_compile_error() - .into(); - } - let destructive = match attrs.get(0) { - Some(syn::NestedMeta::Meta(syn::Meta::NameValue(namevalue))) => { - if let Some(name) = namevalue.path.get_ident().map(|i| i.to_string()) { - if name == "destructive" { - match &namevalue.lit { - syn::Lit::Bool(v) => v.value, - _ => { - return syn::Error::new_spanned( - &attrs[1], - format!("destructive must be bool {}", name), - ) - .to_compile_error() - .into(); - } - } - } else { - return syn::Error::new_spanned( - &attrs[1], - format!("Unknown argument {}", name), - ) - .to_compile_error() - .into(); - } - } else { - false - } - } - Some(v) => { - return syn::Error::new_spanned(&v, "Unexpected argument") - .to_compile_error() - .into() - } - None => false, - }; - let func = syn::parse_macro_input!(input as syn::ItemFn); - let fident = func.sig.ident.clone(); - let varident = quote::format_ident!("ITEST_{}", fident); - let fidentstrbuf = format!(r#"{}"#, fident); - let fidentstr = syn::LitStr::new(&fidentstrbuf, Span::call_site()); - let testident = if destructive { - quote::format_ident!("{}", "DESTRUCTIVE_TESTS") - } else { - quote::format_ident!("{}", "NONDESTRUCTIVE_TESTS") - }; - let output = quote! { - #[linkme::distributed_slice(#testident)] - #[allow(non_upper_case_globals)] - static #varident : Test = Test { - name: #fidentstr, - f: #fident, - }; - #func - }; - output.into() -} diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 2e2bd374..6900d391 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -537,8 +537,7 @@ fn impl_transaction_test>( } } -#[itest(destructive = true)] -fn transactionality() -> Result<()> { +pub(crate) fn itest_transactionality() -> Result<()> { testinit()?; let mark = get_reboot_mark()?; let cancellable = Some(gio::Cancellable::new()); diff --git a/tests/inst/src/insttestmain.rs b/tests/inst/src/insttestmain.rs index 0101363e..9d6d06b0 100644 --- a/tests/inst/src/insttestmain.rs +++ b/tests/inst/src/insttestmain.rs @@ -10,6 +10,25 @@ mod treegen; // Written by Ignition const DESTRUCTIVE_TEST_STAMP: &str = "/etc/ostree-destructive-test-ok"; +macro_rules! test { + ($f: path) => { + (stringify!($f), $f) + }; +} + +type StaticTest = (&'static str, fn() -> Result<()>); + +const TESTS: &[StaticTest] = &[ + test!(sysroot::itest_sysroot_ro), + test!(sysroot::itest_immutable_bit), + test!(sysroot::itest_tmpfiles), + test!(repobin::itest_basic), + test!(repobin::itest_nofifo), + test!(repobin::itest_extensions), + test!(repobin::itest_pull_basicauth), +]; +const DESTRUCTIVE_TESTS: &[StaticTest] = &[test!(destructive::itest_transactionality)]; + #[derive(Debug, StructOpt)] #[structopt(rename_all = "kebab-case")] #[allow(clippy::enum_variant_names)] @@ -30,18 +49,18 @@ enum NonDestructiveOpts { Args(Vec), } -fn libtest_from_test(t: &'static test::Test) -> test::TestImpl { +fn libtest_from_test(t: &StaticTest) -> test::TestImpl { libtest_mimic::Test { - name: t.name.into(), + name: t.0.into(), kind: "".into(), is_ignored: false, is_bench: false, - data: t, + data: t.1, } } fn run_test(test: &test::TestImpl) -> libtest_mimic::Outcome { - if let Err(e) = (test.data.f)() { + if let Err(e) = (test.data)() { libtest_mimic::Outcome::Failed { msg: Some(e.to_string()), } @@ -72,8 +91,8 @@ fn main() -> Result<()> { match opt { Opt::ListDestructive => { - for t in test::DESTRUCTIVE_TESTS.iter() { - println!("{}", t.name); + for t in DESTRUCTIVE_TESTS { + println!("{}", t.0); } Ok(()) } @@ -81,10 +100,7 @@ fn main() -> Result<()> { // FIXME add method to parse subargs let NonDestructiveOpts::Args(iter) = subopt; let libtestargs = libtest_mimic::Arguments::from_iter(iter); - let tests: Vec<_> = test::NONDESTRUCTIVE_TESTS - .iter() - .map(libtest_from_test) - .collect(); + let tests: Vec<_> = TESTS.iter().map(libtest_from_test).collect(); libtest_mimic::run_tests(&libtestargs, tests, run_test).exit(); } Opt::RunDestructive { name } => { @@ -98,10 +114,10 @@ fn main() -> Result<()> { bail!("An ostree-based host is required") } - for t in test::DESTRUCTIVE_TESTS.iter() { - if t.name == name { - (t.f)()?; - println!("ok destructive test: {}", t.name); + for (tname, f) in DESTRUCTIVE_TESTS { + if *tname == name.as_str() { + (f)()?; + println!("ok destructive test: {}", tname); return Ok(()); } } diff --git a/tests/inst/src/repobin.rs b/tests/inst/src/repobin.rs index 2180e81f..582b0290 100644 --- a/tests/inst/src/repobin.rs +++ b/tests/inst/src/repobin.rs @@ -8,15 +8,13 @@ use anyhow::{Context, Result}; use sh_inline::{bash, bash_command}; use with_procspawn_tempdir::with_procspawn_tempdir; -#[itest] -fn test_basic() -> Result<()> { +pub(crate) fn itest_basic() -> Result<()> { bash!(r"ostree --help >/dev/null")?; Ok(()) } -#[itest] #[with_procspawn_tempdir] -fn test_nofifo() -> Result<()> { +pub(crate) fn itest_nofifo() -> Result<()> { assert!(std::path::Path::new(".procspawn-tmpdir").exists()); bash!( r"ostree --repo=repo init --mode=archive @@ -34,9 +32,8 @@ fn test_nofifo() -> Result<()> { Ok(()) } -#[itest] #[with_procspawn_tempdir] -fn test_mtime() -> Result<()> { +pub(crate) fn itest_mtime() -> Result<()> { bash!( r"ostree --repo=repo init --mode=archive mkdir tmproot @@ -50,17 +47,15 @@ fn test_mtime() -> Result<()> { Ok(()) } -#[itest] #[with_procspawn_tempdir] -fn test_extensions() -> Result<()> { +pub(crate) fn itest_extensions() -> Result<()> { bash!(r"ostree --repo=repo init --mode=bare")?; assert!(Path::new("repo/extensions").exists()); Ok(()) } -#[itest] #[with_procspawn_tempdir] -fn test_pull_basicauth() -> Result<()> { +pub(crate) fn itest_pull_basicauth() -> Result<()> { let opts = TestHttpServerOpts { basicauth: true, ..Default::default() diff --git a/tests/inst/src/sysroot.rs b/tests/inst/src/sysroot.rs index b10dbcd4..818b4eb1 100644 --- a/tests/inst/src/sysroot.rs +++ b/tests/inst/src/sysroot.rs @@ -13,8 +13,7 @@ fn skip_non_ostree_host() -> bool { !std::path::Path::new("/run/ostree-booted").exists() } -#[itest] -fn test_sysroot_ro() -> Result<()> { +pub(crate) fn itest_sysroot_ro() -> Result<()> { // TODO add a skipped identifier if skip_non_ostree_host() { return Ok(()); @@ -39,8 +38,7 @@ fn test_sysroot_ro() -> Result<()> { Ok(()) } -#[itest] -fn test_immutable_bit() -> Result<()> { +pub(crate) fn itest_immutable_bit() -> Result<()> { if skip_non_ostree_host() { return Ok(()); } @@ -49,8 +47,7 @@ fn test_immutable_bit() -> Result<()> { Ok(()) } -#[itest] -fn test_tmpfiles() -> Result<()> { +pub(crate) fn itest_tmpfiles() -> Result<()> { if skip_non_ostree_host() { return Ok(()); } diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 81592f7a..6f7aa258 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -6,10 +6,8 @@ use std::process::Command; use std::time; use anyhow::{bail, Context, Result}; -use linkme::distributed_slice; use rand::Rng; -pub use itest_macro::itest; pub use with_procspawn_tempdir::with_procspawn_tempdir; // HTTP Server deps @@ -20,19 +18,7 @@ use hyper_staticfile::Static; use tokio::runtime::Runtime; pub(crate) type TestFn = fn() -> Result<()>; - -#[derive(Debug)] -pub(crate) struct Test { - pub(crate) name: &'static str, - pub(crate) f: TestFn, -} - -pub(crate) type TestImpl = libtest_mimic::Test<&'static Test>; - -#[distributed_slice] -pub(crate) static NONDESTRUCTIVE_TESTS: [Test] = [..]; -#[distributed_slice] -pub(crate) static DESTRUCTIVE_TESTS: [Test] = [..]; +pub(crate) type TestImpl = libtest_mimic::Test; /// Run command and assert that its stderr contains pat pub(crate) fn cmd_fails_with>(mut c: C, pat: &str) -> Result<()> { -- 2.30.2